Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable#320
Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable#320swissspidy with Copilot wants to merge 25 commits into
wp db query and wp db import when mysql/mariadb binary is unavailable#320Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…lable Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/8996fc65-2840-4792-8cf4-ddc700a7c040 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/8996fc65-2840-4792-8cf4-ddc700a7c040 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
wp db query when mysql/mariadb binary is unavailable
This comment was marked as resolved.
This comment was marked as resolved.
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/2fe9988f-095f-4c61-b162-5bf4f8956cca Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
wp db query when mysql/mariadb binary is unavailablewp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailable
This comment was marked as resolved.
This comment was marked as resolved.
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/fa41acc5-de2a-4dc3-9f7a-f5b282ed670a Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
wp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailablewp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailable
…query/import Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…_import() Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…y output Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…...*/) Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…p comments Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
📝 WalkthroughWalkthrough
ChangesMySQL CLI fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant WPCLI
participant DB_Command
participant wpdb
participant Database
WPCLI->>DB_Command: run db query or db import
DB_Command->>DB_Command: detect missing MySQL/MariaDB binary
DB_Command->>wpdb: execute query or split import statements
wpdb->>Database: send SQL
Database-->>wpdb: return rows, status, or errors
wpdb-->>DB_Command: return execution results
DB_Command-->>WPCLI: print output and fallback warning
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/DB_Command.php (1)
1977-1997: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUses a fresh connection per DDL call — fine for single calls, costly in
clean()'s loop.
run_query()now routes everyrun_query()caller through this method when the binary is unavailable, includingclean(), which callsrun_query()once per table (Lines 235-244). Each call opens and closes a brand-new mysqli connection just to run oneDROP TABLE. For large installs with many tables this multiplies connection overhead unnecessarily.♻️ Suggested direction
Accept an optional existing
mysqliconnection (or expose a connection-reuse variant) so callers that issue many DDL statements in a loop (likeclean()) can reuse a single connection instead of opening one per statement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 1977 - 1997, Update run_query_via_mysqli() to accept an optional existing mysqli connection, creating and closing its own connection only when one is not provided. Modify clean() to obtain one reusable connection before its table-drop loop and pass it to each run_query_via_mysqli() call, then close it after the loop while preserving existing error handling.features/db-query.feature (1)
158-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScenario correctly matches the fallback's debug message; consider adding SQL-mode-compat coverage.
This scenario is well-formed and the previously requested
env PATH=...fix is already applied. Given the critical SQL-mode-compatibility gap identified inwpdb_query()/wpdb_import()(src/DB_Command.php), consider adding a fallback scenario analogous to "wp db queryadapts the SQL mode by default without a separate mode probe" (Lines 100-113) but using the fake-bin technique, so a fix to that gap is actually exercised by CI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/db-query.feature` around lines 158 - 183, Add a fake-bin fallback scenario in features/db-query.feature that verifies wp db query adapts SQL mode by default without a separate mode probe, using the same PATH override and unavailable mysql/mariadb binaries as the current scenario. Ensure the assertions exercise the compatibility behavior implemented by DB_Command::wpdb_query() or DB_Command::wpdb_import(), including the expected query result and debug output.features/db-import.feature (1)
72-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScenario structure is correct but doesn't reproduce the known regression.
This test imports a plain export on what is likely a lenient-SQL-mode CI database, so it wouldn't fail even with the missing SQL-mode-compat statement in
wpdb_import()(see src/DB_Command.php). Consider mirroring "wp db importloads a dump containing legacy zero-date values" (Lines 323-344) but through the fake-bin/mysqli fallback path, since that's the exact scenario that previously surfaced "Invalid default value for 'comment_date'" per the PR history.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/db-import.feature` around lines 72 - 99, The fallback scenario should import a dump containing legacy zero-date values so it exercises the SQL-mode compatibility logic in DB_Command::wpdb_import(), rather than importing a plain export. Adapt the existing fake-bin/mysql and fake-bin/mariadb unavailable setup to use the zero-date dump content and retain assertions for successful import and the fallback warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/DB_Command.php`:
- Around line 2593-2629: Apply get_sql_mode_compat_statement( $assoc_args ) to
the fallback connection before executing SQL in both src/DB_Command.php lines
2593-2629 within wpdb_import() and lines 2535-2583 within wpdb_query(); each
site requires the same compatibility statement before the split statements or
query runs.
- Around line 2008-2013: Update the run_query() fallback around
is_mysql_binary_available() so database creation does not route through
run_query_via_mysqli() without a selected database. Exclude the create()
operation from this fallback, preserving the existing mysqli behavior for other
queries, or explicitly align the command documentation and design with
supporting create() through that path.
- Around line 2481-2524: Update get_db_connection() to use the command’s
effective CLI database username and password values, including --dbuser and
--dbpass overrides, instead of always reading DB_USER and DB_PASSWORD. Handle
mysqli connection exceptions so failures reach the existing WP_CLI::error()
path, and ensure fallback query operations similarly tolerate or catch mysqli
exceptions before relying on connect_errno/query checks.
---
Nitpick comments:
In `@features/db-import.feature`:
- Around line 72-99: The fallback scenario should import a dump containing
legacy zero-date values so it exercises the SQL-mode compatibility logic in
DB_Command::wpdb_import(), rather than importing a plain export. Adapt the
existing fake-bin/mysql and fake-bin/mariadb unavailable setup to use the
zero-date dump content and retain assertions for successful import and the
fallback warning.
In `@features/db-query.feature`:
- Around line 158-183: Add a fake-bin fallback scenario in
features/db-query.feature that verifies wp db query adapts SQL mode by default
without a separate mode probe, using the same PATH override and unavailable
mysql/mariadb binaries as the current scenario. Ensure the assertions exercise
the compatibility behavior implemented by DB_Command::wpdb_query() or
DB_Command::wpdb_import(), including the expected query result and debug output.
In `@src/DB_Command.php`:
- Around line 1977-1997: Update run_query_via_mysqli() to accept an optional
existing mysqli connection, creating and closing its own connection only when
one is not provided. Modify clean() to obtain one reusable connection before its
table-drop loop and pass it to each run_query_via_mysqli() call, then close it
after the loop while preserving existing error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 147dfdef-c9e6-4d69-9dda-8223e5fcbc7f
📒 Files selected for processing (4)
features/db-import.featurefeatures/db-query.featurefeatures/db.featuresrc/DB_Command.php
wp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailablewp db query and wp db import when mysql/mariadb binary is unavailable
a7665f9 to
30dfd4f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/DB_Command.php (1)
602-619: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDocument the wpdb-fallback behavior and its limitations in the
query/importdocblocks. Both commands gained a significant new code path (automatic fallback to$wpdbwhen the mysql/mariadb binary is unavailable) with real behavioral differences from the mysql-client path, but neither docblock mentions it.
src/DB_Command.php#L602-L619: add a note toquery()'s docblock that when the mysql/mariadb binary is unavailable, the query runs through$wpdbinstead,--dbuser/--dbpass/other mysql-specific options are not honored in that mode, and multi-statement input (e.g.wp db query < file.sql) is not currently supported via this fallback.src/DB_Command.php#L955-L977: add a note toimport()'s docblock that the wpdb fallback ignores--dbuser/--dbpass, and that dumps relying on customDELIMITERblocks (stored routines/triggers) or server-level DDL are not supported by the fallback.As per path instructions,
**/*.php: "Update relevant inline code documentation when changes affect user-facing functionality."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 602 - 619, Update the query() docblock at src/DB_Command.php lines 602-619 to document that unavailable mysql/mariadb binaries trigger execution through $wpdb, mysql-specific options including --dbuser and --dbpass are ignored, and multi-statement input is unsupported. Update the import() docblock at src/DB_Command.php lines 955-977 to document that the fallback ignores --dbuser/--dbpass and does not support custom DELIMITER blocks or server-level DDL.Source: Coding guidelines
♻️ Duplicate comments (2)
src/DB_Command.php (2)
2501-2507: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--dbuser/--dbpass(and other connection overrides) are silently ignored by the wpdb fallback.
maybe_load_wpdb()always instantiateswpdbwith the rawDB_USER/DB_PASSWORD/DB_HOSTconstants, with no way to honor--dbuser/--dbpasspassed towp db query/wp db import. This is the same underlying gap flagged on the previous mysqli-based implementation (get_db_connection()readingDB_USER/DB_PASSWORDdirectly), just carried into the new wpdb-based fallback.🛠️ Suggested direction
- protected function maybe_load_wpdb() { + protected function maybe_load_wpdb( $assoc_args = [] ) { ... - $wpdb = new wpdb( DB_USER, DB_PASSWORD, DB_NAME, DB_HOST ); + $dbuser = Utils\get_flag_value( $assoc_args, 'dbuser', DB_USER ); + $dbpass = Utils\get_flag_value( $assoc_args, 'dbpass', DB_PASSWORD ); + $wpdb = new wpdb( $dbuser, $dbpass, DB_NAME, DB_HOST );and pass
$assoc_argsthrough fromwpdb_query()/wpdb_import().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2501 - 2507, Update maybe_load_wpdb() to accept the connection override arguments and use them when constructing the fallback wpdb instance instead of always using DB_USER, DB_PASSWORD, DB_NAME, and DB_HOST directly. Pass $assoc_args through from wpdb_query() and wpdb_import(), preserving the constants as defaults when overrides are absent.
2585-2606: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winSQL-mode compatibility statement is missing from both wpdb fallback executors. Neither
wpdb_import()norwpdb_query()appliesget_sql_mode_compat_statement( $assoc_args )on the fallback connection, unlike the CLI path's--init-command. This is the same root cause previously flagged, and matches the PR history's reported persisting "Invalid default value for 'comment_date'" import failure across WordPress versions.
src/DB_Command.php#L2585-L2606: inwpdb_import(), run$this->get_sql_mode_compat_statement( $assoc_args )via$wpdb->query()before executing the split statements.src/DB_Command.php#L2519-L2575: inwpdb_query(), run the same compatibility statement before executing$query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2585 - 2606, Apply the SQL-mode compatibility statement to both fallback executors before running SQL: in src/DB_Command.php lines 2585-2606, update wpdb_import() to execute get_sql_mode_compat_statement( $assoc_args ) through $wpdb->query() before the split statements; likewise in src/DB_Command.php lines 2519-2575, update wpdb_query() to execute the same statement before $query.
🧹 Nitpick comments (1)
src/DB_Command.php (1)
2656-2751: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStatement splitter doesn't handle custom
DELIMITERblocks (stored routines/triggers).
split_sql_statements()correctly tracks quotes/comments/conditional comments, but has no concept of a customDELIMITER(e.g.DELIMITER ;;used by dumps with--routines/--triggers). Any embedded;inside such a routine body would be split as a statement boundary, corrupting the routine definition. Given--skip-optimizationaside, this is a real gap for non-trivial dumps, though likely acceptable if this limitation is documented (see the docblock consolidation comment) rather than fixed outright, since the fallback already explicitly doesn't support server-level DDL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2656 - 2751, Update split_sql_statements() to recognize DELIMITER directives and use the configured delimiter instead of treating every semicolon inside stored routine or trigger definitions as a statement boundary. Preserve existing quote, comment, and conditional-comment handling, and ensure DELIMITER directives themselves are not included in returned SQL statements; if this scope is intentionally unsupported, document that limitation in the associated method docblock instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/DB_Command.php`:
- Around line 2519-2575: Update wpdb_query() to pass the input through the
existing split_sql_statements() helper and execute each resulting statement
individually, matching the iteration behavior used by wpdb_import(). Preserve
per-statement handling for modifying queries, errors, affected-row success
messages, result headers, and rows, while ensuring all statements from piped SQL
are processed.
- Around line 2519-2526: Update wpdb_query() to suppress wpdb errors during
query execution, mirroring the behavior in wpdb_import(). Capture the previous
suppression state, call $wpdb->suppress_errors(true) before executing the query,
and restore the captured state before every return path and when the method
completes.
- Around line 2633-2645: Update the transaction cleanup in the import flow
around the $wpdb query calls and its enclosing method so failures from COMMIT or
any settings-restore query are detected and propagated to the caller. Ensure
import() does not print its success message when cleanup fails, while preserving
normal success behavior when all queries complete successfully.
- Around line 2480-2493: Update the wpdb bootstrap required-files list near
$required_files to include WordPress's wp-includes/functions.php before loading
dependent files. Keep the existing require_once loop and file existence checks
unchanged so wpdb::print_error() and get_caller() can safely resolve
wp_debug_backtrace_summary().
---
Outside diff comments:
In `@src/DB_Command.php`:
- Around line 602-619: Update the query() docblock at src/DB_Command.php lines
602-619 to document that unavailable mysql/mariadb binaries trigger execution
through $wpdb, mysql-specific options including --dbuser and --dbpass are
ignored, and multi-statement input is unsupported. Update the import() docblock
at src/DB_Command.php lines 955-977 to document that the fallback ignores
--dbuser/--dbpass and does not support custom DELIMITER blocks or server-level
DDL.
---
Duplicate comments:
In `@src/DB_Command.php`:
- Around line 2501-2507: Update maybe_load_wpdb() to accept the connection
override arguments and use them when constructing the fallback wpdb instance
instead of always using DB_USER, DB_PASSWORD, DB_NAME, and DB_HOST directly.
Pass $assoc_args through from wpdb_query() and wpdb_import(), preserving the
constants as defaults when overrides are absent.
- Around line 2585-2606: Apply the SQL-mode compatibility statement to both
fallback executors before running SQL: in src/DB_Command.php lines 2585-2606,
update wpdb_import() to execute get_sql_mode_compat_statement( $assoc_args )
through $wpdb->query() before the split statements; likewise in
src/DB_Command.php lines 2519-2575, update wpdb_query() to execute the same
statement before $query.
---
Nitpick comments:
In `@src/DB_Command.php`:
- Around line 2656-2751: Update split_sql_statements() to recognize DELIMITER
directives and use the configured delimiter instead of treating every semicolon
inside stored routine or trigger definitions as a statement boundary. Preserve
existing quote, comment, and conditional-comment handling, and ensure DELIMITER
directives themselves are not included in returned SQL statements; if this scope
is intentionally unsupported, document that limitation in the associated method
docblock instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a2840be-db42-4924-83dc-02975a8da892
📒 Files selected for processing (3)
features/db-import.featurefeatures/db.featuresrc/DB_Command.php
💤 Files with no reviewable changes (1)
- features/db.feature
🚧 Files skipped from review as they are similar to previous changes (1)
- features/db-import.feature
| // Load prerequisite WordPress files if not already loaded. | ||
| $required_files = [ | ||
| ABSPATH . WPINC . '/load.php', | ||
| ABSPATH . WPINC . '/compat.php', | ||
| ABSPATH . WPINC . '/plugin.php', | ||
| ABSPATH . WPINC . '/class-wp-error.php', | ||
| $wpdb_file, | ||
| ]; | ||
|
|
||
| foreach ( $required_files as $required_file ) { | ||
| if ( file_exists( $required_file ) ) { | ||
| require_once $required_file; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect maybe_load_wpdb implementation and nearby context.
rg -n "maybe_load_wpdb|required_files|class-wp-error|load\.php|compat\.php|plugin\.php|functions\.php" src/DB_Command.php
printf '\n---\n'
sed -n '2440,2515p' src/DB_Command.php
printf '\n===\n'
# Search for direct references to helpers implicated by the review comment.
rg -n "_doing_it_wrong\(|is_multisite\(" src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 2962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search repository for any local compatibility handling around wpdb/core bootstrap.
rg -n "maybe_load_wpdb|wpdb_file|wp-includes/functions\.php|_doing_it_wrong\(|is_multisite\(" .Repository: wp-cli/db-command
Length of output: 543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the WordPress core version references vendored or mentioned in the repo, if any.
git ls-files | rg '(^|/)wp-includes/|(^|/)wordpress/|(^|/)tests?/.*wordpress|wp-cli'Repository: wp-cli/db-command
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify whether the repo includes tests or docs discussing maybe_load_wpdb failure modes.
rg -n "maybe_load_wpdb|fallback reported|WordPress core files|wpdb" .Repository: wp-cli/db-command
Length of output: 9836
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- DB_Command.php excerpt ---'
nl -ba src/DB_Command.php | sed -n '2468,2505p'
echo
echo '--- search for helper calls in DB_Command.php ---'
rg -n "_doing_it_wrong\(|is_multisite\(|wpdb->prepare|get_results\(|query\(" src/DB_Command.php
echo
echo '--- search for related bootstrap / core-file mentions in repo ---'
rg -n "functions\.php|load\.php|compat\.php|plugin\.php|class-wp-error\.php|maybe_load_wpdb|wpdb_file" .
echo
echo '--- repo file inventory for tests/docs mentioning wpdb or core bootstrap ---'
git ls-files | rg '(test|spec|doc|readme|md)$|DB_Command|wpdb|wordpress'Repository: wp-cli/db-command
Length of output: 226
🌐 Web query:
WordPress wpdb::prepare _doing_it_wrong functions.php load.php is_multisite class-wpdb.php
💡 Result:
In WordPress, the _doing_it_wrong function is used to notify developers that a specific function or method is being called incorrectly [1]. When you see an error related to wpdb::prepare and _doing_it_wrong, it indicates that the wpdb::prepare method—found in wp-includes/class-wpdb.php—has been used in a way that violates safety or API standards [2][3]. Common causes for this error include: 1. Missing Placeholders: The most frequent cause is calling wpdb::prepare with a query string that contains no placeholders (e.g., %s, %d, %f) [2][4]. WordPress requires that any query passed to this method must include at least one placeholder to ensure the data is properly escaped [2][5]. 2. Insufficient Arguments: The method expects a matching argument for every placeholder present in the query [6][3]. Providing fewer arguments than there are placeholders will trigger this notice [5][6]. 3. Mismatched Placeholders/Arguments: If a query contains multiple placeholders but the passed arguments do not align correctly (or if an array is used incorrectly), the system flags the usage [6][3]. Regarding the other components you mentioned: - wp-includes/functions.php: This is the file where the _doing_it_wrong function itself is defined [1][7]. It handles triggering the notification (often via the doing_it_wrong_run action or by outputting a warning in debug mode) [1]. - wp-includes/load.php: This file contains low-level functions required early in the WordPress lifecycle, such as is_multisite [8][9]. While not directly responsible for the wpdb::prepare logic, it contains helper functions like wp_load_translations_early which may be called by wpdb::prepare to ensure the _doing_it_wrong message is translatable [2][3]. - is_multisite: This function determines whether a WordPress site is running in a Multisite configuration [8]. It is defined in wp-includes/load.php [8]. It is not directly related to the logic of wpdb::prepare placeholders, but both are core components often involved in the early boot process of WordPress [3][8]. To resolve a "Function wpdb::prepare was called incorrectly" error, review the SQL query string passed as the first argument to ensure it contains at least one placeholder, and verify that every placeholder has a corresponding, correctly formatted argument [4]. You can use WP_DEBUG to display these notices during development [1].
Citations:
- 1: https://developer.wordpress.org/reference/functions/_doing_it_wrong/
- 2: https://developer.wordpress.org/reference/classes/wpdb/prepare/
- 3: https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wpdb.php
- 4: https://wordpress.stackexchange.com/questions/426869/function-wpdbprepare-was-called-incorrectly-the-query-argument-of-wpdbprepa
- 5: https://core.trac.wordpress.org/ticket/22262
- 6: https://developer.wordpress.org/reference/classes/wpdb/prepare/?output_format=md
- 7: https://developer.wordpress.org/reference/functions/wp_maybe_update_user_counts/
- 8: https://developer.wordpress.org/reference/functions/is_multisite/
- 9: https://github.com/WordPress/WordPress/blob/master/wp-includes/load.php
🌐 Web query:
site:github.com/WordPress/WordPress class-wpdb.php _doing_it_wrong functions.php
💡 Result:
The _doing_it_wrong function calls found within class-wpdb.php are triggered when the $wpdb database class detects improper usage of its methods, particularly wpdb::prepare or database escaping functions [1][2][3]. These warnings are intended for developers to help identify and fix insecure or technically incorrect code during development [1][3]. Common scenarios that trigger these warnings include: 1. Improper use of wpdb::prepare: - Missing placeholders: If a query passed to wpdb::prepare does not contain any placeholders (e.g., %), it triggers a warning because it suggests the query might not be properly sanitized [1][4][3]. - Unsupported value types: If the arguments passed to wpdb::prepare are not scalar or null (e.g., passing an object or array where a string or integer is expected), a warning is issued [5][6][3]. - Placeholder mismatch: If the number of provided arguments does not match the placeholders in the query string, a warning is triggered to prevent potential errors [1][4][3]. 2. Database escaping issues: - Missing database connection: The _real_escape method triggers a _doing_it_wrong warning if it is called before a database connection has been established, indicating an attempt to escape data without the necessary context [2][7][6]. When these warnings occur, they are typically logged to the debug.log file if WP_DEBUG and WP_DEBUG_LOG are enabled. To resolve them, you should examine the stack trace provided in the error message to locate the specific code (often in your theme's functions.php or a plugin file) that is calling the $wpdb method incorrectly and update it to follow WordPress coding standards, such as ensuring all SQL queries use proper placeholders [1][3].
Citations:
- 1: https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wpdb.php
- 2: https://github.com/WordPress/WordPress/blob/4.8.3/wp-includes/wp-db.php
- 3: https://github.com/WordPress/WordPress/blob/807cba060e30a6707aa7ffc7baafa60da4b4e0ba/wp-includes/wp-db.php
- 4: https://github.com/WordPress/WordPress/blob/14423850/wp-includes/class-wpdb.php
- 5: https://github.com/WordPress/WordPress/blob/9e0f2faa28f1aa04a69e0b4eaa410a38adcd2e1e/wp-includes/class-wpdb.php
- 6: https://github.com/WordPress/WordPress/blob/5.3.2/wp-includes/wp-db.php
- 7: https://github.com/WordPress/WordPress/blob/8e08c569c512932b0802c70c3bf786483ddd10b5/wp-includes/wp-db.php
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the SQLite bootstrap because it may show which core helpers wpdb actually needs.
sed -n '540,590p' src/DB_Command_SQLite.phpRepository: wp-cli/db-command
Length of output: 1510
🌐 Web query:
WordPress wpdb wp_debug_backtrace_summary functions.php class-wpdb.php query print_error
💡 Result:
In WordPress, the relationship between wpdb, wp_debug_backtrace_summary, and print_error is central to database debugging. wp_debug_backtrace_summary is a utility function located in wp-includes/functions.php that generates a comma-separated string (or array) of function names in the current backtrace [1][2][3]. It is designed to provide context for debugging by showing the call stack up to the current point [3]. The wpdb class leverages this function for internal diagnostics [4][5]: 1. wpdb::get_caller: This internal method calls wp_debug_backtrace_summary(CLASS) to retrieve the name of the function(s) that triggered a specific database query [4]. 2. wpdb::print_error: When a database error occurs, this method uses wpdb::get_caller to identify where the offending query originated [6]. It then includes this "made by [caller]" information in the database error log entry [6]. Key debugging components: - wpdb::print_error(string $str = ''): This method prints or logs SQL/database errors [6]. If database errors are suppressed (via $wpdb->suppress_errors), it returns false [6]. If not, it logs the error message (including the caller's identity) via error_log [6]. On non-multisite installations, it also attempts to display the error directly on the screen if $wpdb->show_errors is enabled [6]. - SAVEQUERIES: If this constant is set to true in wp-config.php, WordPress tracks all queries in the global $wpdb->queries array [7][8]. This log includes the query SQL, execution time, and a comma-separated list of calling functions (generated using the same logic as the backtrace tools) [7]. In summary, wp_debug_backtrace_summary provides the trace data, which wpdb::get_caller extracts for use by wpdb::print_error to create detailed, debuggable error messages that identify exactly which part of your code initiated a failed query [4][6].
Citations:
- 1: https://developer.wordpress.org/reference/functions/wp_debug_backtrace_summary/
- 2: https://www.typeerror.org/docs/wordpress/functions/wp_debug_backtrace_summary
- 3: https://westi.wordpress.com/2012/01/28/tracing-things-back-to-where-they-came-from/
- 4: https://developer.wordpress.org/reference/classes/wpdb/get_caller/
- 5: https://wpseek.com/source/wp/latest/_functions/wp_debug_backtrace_summary.html
- 6: https://developer.wordpress.org/reference/classes/wpdb/print_error/
- 7: https://developer.wordpress.org/reference/classes/wpdb/
- 8: https://developer.wordpress.org/advanced-administration/debug/debug-wordpress/?output_format=md
Load functions.php in the wpdb bootstrap load.php already covers is_multisite(), but wpdb::print_error()/get_caller() rely on wp_debug_backtrace_summary() from wp-includes/functions.php, so a query failure can still fatal without it.
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 2491-2491: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
🪛 PHPStan (2.2.5)
[error] 2482-2482: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
[error] 2483-2483: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
[error] 2484-2484: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
[error] 2485-2485: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 2480 - 2493, Update the wpdb bootstrap
required-files list near $required_files to include WordPress's
wp-includes/functions.php before loading dependent files. Keep the existing
require_once loop and file existence checks unchanged so wpdb::print_error() and
get_caller() can safely resolve wp_debug_backtrace_summary().
| protected function wpdb_query( $query, $assoc_args = [] ) { | ||
| $this->maybe_load_wpdb(); | ||
| global $wpdb; | ||
|
|
||
| if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) { | ||
| WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
wpdb_query() doesn't suppress wpdb's default error output, unlike wpdb_import().
wpdb_import() wraps execution in $wpdb->suppress_errors( true ), but wpdb_query() never does. With WordPress's default wpdb::$show_errors = true, a failing query can trigger wpdb's own PHP warning/HTML error output before WP_CLI::error() prints the clean message, leaking noisy output into the CLI.
🛠️ Proposed fix
protected function wpdb_query( $query, $assoc_args = [] ) {
$this->maybe_load_wpdb();
global $wpdb;
if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) {
WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' );
}
+ $suppress = $wpdb->suppress_errors( true );
+
$skip_column_names = Utils\get_flag_value( $assoc_args, 'skip-column-names', false );(and restore $wpdb->suppress_errors( $suppress ); before each return path / at the end of the method, mirroring wpdb_import().)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 2519 - 2526, Update wpdb_query() to suppress
wpdb errors during query execution, mirroring the behavior in wpdb_import().
Capture the previous suppression state, call $wpdb->suppress_errors(true) before
executing the query, and restore the captured state before every return path and
when the method completes.
| protected function wpdb_query( $query, $assoc_args = [] ) { | ||
| $this->maybe_load_wpdb(); | ||
| global $wpdb; | ||
|
|
||
| if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) { | ||
| WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' ); | ||
| } | ||
|
|
||
| $skip_column_names = Utils\get_flag_value( $assoc_args, 'skip-column-names', false ); | ||
| $is_row_modifying_query = (bool) preg_match( '/\b(UPDATE|DELETE|INSERT|REPLACE(?!\s*\()|LOAD DATA)\b/i', $query ); | ||
|
|
||
| if ( $is_row_modifying_query ) { | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $affected_rows = $wpdb->query( $query ); | ||
| if ( false === $affected_rows ) { | ||
| // phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags | ||
| WP_CLI::error( 'Query failed: ' . strip_tags( $wpdb->last_error ) ); | ||
| } | ||
| WP_CLI::success( "Query succeeded. Rows affected: {$affected_rows}" ); | ||
| } else { | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $results = $wpdb->get_results( $query, ARRAY_A ); | ||
|
|
||
| if ( $wpdb->last_error ) { | ||
| // phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags | ||
| WP_CLI::error( 'Query failed: ' . strip_tags( $wpdb->last_error ) ); | ||
| } | ||
|
|
||
| if ( empty( $results ) ) { | ||
| return; | ||
| } | ||
|
|
||
| $headers = array_keys( $results[0] ); | ||
| if ( ! $skip_column_names && ! empty( $headers ) ) { | ||
| WP_CLI::line( implode( "\t", $headers ) ); | ||
| } | ||
| foreach ( $results as $row ) { | ||
| WP_CLI::line( | ||
| implode( | ||
| "\t", | ||
| array_map( | ||
| static function ( $v ) { | ||
| if ( null === $v ) { | ||
| return 'NULL'; | ||
| } | ||
| if ( is_scalar( $v ) ) { | ||
| return (string) $v; | ||
| } | ||
| return ''; | ||
| }, | ||
| array_values( $row ) | ||
| ) | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
wpdb_query() doesn't split multi-statement SQL, unlike wpdb_import().
The command's own docs show piping a file into wp db query ($ wp db query < debug.sql), which commonly contains multiple ;-terminated statements. wpdb_query() passes the entire string straight to a single $wpdb->query( $query ) / $wpdb->get_results( $query, ARRAY_A ) call. WordPress's wpdb connects without MYSQLI_CLIENT_MULTI_STATEMENTS, so a multi-statement string will typically fail or silently execute only the first statement — unlike the real mysql binary path, and unlike wpdb_import(), which already reuses split_sql_statements() for exactly this reason.
🛠️ Suggested direction
Reuse split_sql_statements() in wpdb_query() and iterate over the resulting statements the same way wpdb_import() does, aggregating/echoing results as each statement executes.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 2531-2531: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( $query )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
🪛 PHPStan (2.2.5)
[error] 2540-2540: Constant ARRAY_A not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
[error] 2551-2551: Parameter #1 $array of function array_keys expects array, array|stdClass given.
(argument.type)
[error] 2569-2569: Parameter #1 $array of function array_values expects array, array|stdClass given.
(argument.type)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 2519 - 2575, Update wpdb_query() to pass the
input through the existing split_sql_statements() helper and execute each
resulting statement individually, matching the iteration behavior used by
wpdb_import(). Preserve per-statement handling for modifying queries, errors,
affected-row success messages, result headers, and rows, while ensuring all
statements from piped SQL are processed.
| if ( ! $skip_optimization ) { | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $wpdb->query( 'COMMIT' ); | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $wpdb->query( 'SET autocommit = 1' ); | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $wpdb->query( 'SET unique_checks = 1' ); | ||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $wpdb->query( 'SET foreign_key_checks = 1' ); | ||
| } | ||
|
|
||
| $wpdb->suppress_errors( $suppress ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
COMMIT/settings-restore failures after import are not checked — the caller reports success anyway.
If $wpdb->query( 'COMMIT' ) fails (lock timeout, dropped connection, etc.), the function returns normally with no error signaled. import() then unconditionally prints Success: Imported from '...'., even though the transaction may not have been committed — risking a false "success" report and silent data loss for the whole import.
🛠️ Proposed fix
if ( ! $skip_optimization ) {
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
- $wpdb->query( 'COMMIT' );
+ $commit_result = $wpdb->query( 'COMMIT' );
+ if ( false === $commit_result ) {
+ $error = $wpdb->last_error;
+ $wpdb->suppress_errors( $suppress );
+ // phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags
+ WP_CLI::error( 'Import failed to commit: ' . strip_tags( $error ) );
+ }
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$wpdb->query( 'SET autocommit = 1' );🧰 Tools
🪛 ast-grep (0.44.1)
[error] 2634-2634: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'COMMIT' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
[error] 2636-2636: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET autocommit = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
[error] 2638-2638: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET unique_checks = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
[error] 2640-2640: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET foreign_key_checks = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 2633 - 2645, Update the transaction cleanup
in the import flow around the $wpdb query calls and its enclosing method so
failures from COMMIT or any settings-restore query are detected and propagated
to the caller. Ensure import() does not print its success message when cleanup
fails, while preserving normal success behavior when all queries complete
successfully.
wp db queryandwp db importcurrently require themysql/mariadbCLI binary, causing failures with drop-in database engines (HyperDB, custom drivers) or environments where the binary isn't installed. This PR adds automatic fallback to WordPress's$wpdbfor these commands when the binary is unavailable.Changes
src/DB_Command.phpis_mysql_binary_available()— probes for themysql/mariadbbinary viaProcess::create(); result is statically cached.maybe_load_wpdb()— loads minimal WordPress files needed for$wpdb(load.php,compat.php,plugin.php,functions.php,class-wpdb.php), includes anywp-content/db.phpdrop-in (e.g. HyperDB), and creates a plainwpdbinstance withwp-config.phpcredentials and$table_prefixif$wpdbis not already initialized.query()— checks binary availability after SQLite handling; routes towpdb_query()when absent.wpdb_query()— executes SQL via$wpdb; formats SELECT results as tab-separated values matching themysqlCLI output format, and reports affected rows for DML operations.import()— when the mysql binary is unavailable, reads SQL from file or STDIN and delegates towpdb_import().wpdb_import()— executes SQL statement-by-statement through$wpdbwith transaction and constraint optimization toggles (autocommit=0,unique_checks=0,foreign_key_checks=0), suppressing$wpdboutput and ignoring non-essential privilege errors onSETstatements (e.g.SQL_LOG_BIN,GTID).split_sql_statements()— state-machine SQL parser handling single/double-quoted strings, line comments (--), block comments (/* */), and MySQL conditional comments (/*!...*/).Tests
@require-mysql-or-mariadbfallback scenarios infeatures/db-query.featureandfeatures/db-import.featureasserting fallback to$wpdbwhen binaries are shadowed.Summary by CodeRabbit
New Features
wp db querynow falls back to WordPress database execution when MySQL/MariaDB client tools are unavailable.wp db importcan import SQL files or standard input without requiring MySQL/MariaDB client binaries.Bug Fixes